Skip to content

Allow for more than 1000 char input per line for Exercise 1-17 in Chapter 1 #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

jerryq0101
Copy link
Contributor

Tested with sample input by defining an EXIT CHAR in terminal.

Would love to get feedback on C code. Let me know if there are non optimal practices used, or if its incorrect.

@jerryq0101 jerryq0101 marked this pull request as ready for review September 25, 2024 01:04
@ohkimur
Copy link
Owner

ohkimur commented Sep 29, 2024

@jerryq0101 First, thanks for your contribution. Second, I'm struggling to understand the value that your PR actually provides beyond the original solution. Can you elaborate a bit so that I understand the motivation behind your PR?

@jerryq0101
Copy link
Contributor Author

Thanks for the reply! Since the question in the book simply asked for printing more than 80 characters, I thought it could be a more complete solution for each line to not be limited by 1000 characters in the original solution.

Lmk what you think.

@ohkimur
Copy link
Owner

ohkimur commented Mar 31, 2025

Hello @jerryq0101! Thanks a lot for your contribution! Sorry for the late review. After looking at the code, I don't see that it provides any useful value for the objective of the exercise. Also, the code is not respecting the style in the repo. Looking at it, maybe having a linter would be useful (PS: maybe you would like to contribute to that). It's nice that we can have arbitrary line lengths, but I propose that we not merge this as is, since it's not very friendly for beginners that only arrived at this point in the book.

I am also strongly against comments. Having them for me is a failure of writing good, readable code. Also, let's not use magic numbers and use constants at the top of the file.

If you think you could improve the code to be more readable for beginners, you can reopen this PR. I'd love to merge it. The idea is good, but it needs refinement.

@ohkimur ohkimur closed this Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants